-
Notifications
You must be signed in to change notification settings - Fork 26
[TASK-250] KV Tables in CPP #288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@luoyuxia @zhaohaidao PTAL 🙏 |
599949f to
9b48b29
Compare
leekeiabstraction
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TY for the PR! Left some questions
|
@leekeiabstraction TY for the review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds KV-table (primary-key) operations to the C++ bindings by introducing upsert/delete and point-lookup APIs, plus required row/FFI conversions to match existing Rust/Python behavior.
Changes:
- Add
UpsertWriterandLookuperto the C++ API and Rust/CXX bridge (create writer, upsert/delete/flush, lookup). - Add schema-aware
GenericRowcreation (Table::NewRow) and name-based setters for more ergonomic row building. - Improve/extend Rust↔FFI row conversions (including TinyInt/SmallInt and safer error propagation), and add a KV usage example + build targets.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| bindings/cpp/src/types.rs | Improves datum conversions (TinyInt/SmallInt, timestamp error handling), returns Result instead of panicking, adds internal_row_to_ffi_row for lookup results. |
| bindings/cpp/src/table.cpp | Implements C++ UpsertWriter/Lookuper, adds Table::NewRow() and schema column-map caching. |
| bindings/cpp/src/lib.rs | Extends the CXX bridge with new KV types/APIs; adds Rust-side UpsertWriter/Lookuper implementations and lookup result wiring. |
| bindings/cpp/src/ffi_converter.hpp | Minor formatting change. |
| bindings/cpp/include/fluss.hpp | Adds schema-aware name-based setters on GenericRow, new KV API declarations. |
| bindings/cpp/examples/kv_example.cpp | New end-to-end KV example covering upsert/delete/lookup and partial updates. |
| bindings/cpp/examples/admin_example.cpp | Minor formatting change. |
| bindings/cpp/CMakeLists.txt | Builds the new KV example binary. |
| bindings/cpp/BUILD.bazel | Adds Bazel target for the KV example binary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Addressed comments |
luoyuxia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fresh-borzoni Thanks. LGTM!
Summary
closes #250
KV Tables Support for C++ Bindings
Added
UpsertWriterandLookuperclasses to the C++ bindings, enabling full KV table operations on primary-key tables.UpsertWriter
NewUpsertWriter(writer, {"user_id", "balance"}))NewUpsertWriter(writer, {0, 1}))Lookuper
Note: currently copies over FFI, will be optimized as part of broader #87